-
Notifications
You must be signed in to change notification settings - Fork 130
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[23] DOM changes for markdown Javadoc comments #2738 #2807
Conversation
@stephan-herrmann , there's a long way to go for this PR. Plenty of new tests still failing. I already see that the flatteners are going to need some changes to the existing DOM Javadoc nodes to differentiate markdown comments. I would like you to try this out and let me know what you would need in terms of the DOM changes for rendering. |
Thanks @jarthana. I do get some markdown Javadoc DOM nodes, great! As soon as links are involved, some details don't yet look right, but I need to take a closer look to find out if that is caused in DOM or during rendering. I'll let you know. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jarthana once you feel somewhat comfortable with existing changes, despite any incompleteness merging this change would let me push some real changes in JDT/UI. That would establish a basis for investigating into details and corner cases.
As mentioned before a new API Javadoc.isMarkdown()
or Javadoc.getCommentStyle()
would allow for a cleaner check, but currently the workaround to test the raw javadoc for leading ///
is functional.
org.eclipse.jdt.core/dom/org/eclipse/jdt/core/dom/DocCommentParser.java
Outdated
Show resolved
Hide resolved
@jarthana the following two changes help a great deal to pass correct diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/parser/AbstractCommentParser.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/parser/AbstractCommentParser.java
index 70a929a..bcba898 100644
--- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/parser/AbstractCommentParser.java
+++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/parser/AbstractCommentParser.java
@@ -504,7 +504,7 @@
}
refreshInlineTagPosition(textEndPosition);
setInlineTagStarted(false);
- } else if (this.lineStarted && this.textStart != -1 && this.textStart <= textEndPosition && (this.textStart < this.starPosition || this.starPosition == lastStarPosition)) {
+ } else if (this.lineStarted && this.textStart != -1 && this.textStart <= textEndPosition && (this.textStart < this.starPosition || this.starPosition == lastStarPosition || this.markdown)) {
pushText(this.textStart, textEndPosition);
}
updateDocComment();
diff --git a/org.eclipse.jdt.core/dom/org/eclipse/jdt/core/dom/DocCommentParser.java b/org.eclipse.jdt.core/dom/org/eclipse/jdt/core/dom/DocCommentParser.java
index c125d44..bdd0b86 100644
--- a/org.eclipse.jdt.core/dom/org/eclipse/jdt/core/dom/DocCommentParser.java
+++ b/org.eclipse.jdt.core/dom/org/eclipse/jdt/core/dom/DocCommentParser.java
@@ -761,7 +761,7 @@
// Both tag elements must get the same source range.
TagElement previousTag = (TagElement) this.astStack[this.astPtr];
int parentStart = previousTag.getStartPosition();
- int length = this.index - previousPosition + 1;
+ int length = this.index - previousPosition ;
previousTag.setSourceRange(parentStart, this.index - parentStart + 1);
List fragments = previousTag.fragments();
int size = fragments.size();
@@ -774,7 +774,7 @@
// If last fragment is a tag, then use it as previous tag
ASTNode lastFragment = (ASTNode) fragments.get(size-1);
if (lastFragment.getNodeType() == ASTNode.TAG_ELEMENT) {
- //lastFragment.setSourceRange(lastFragment.getStartPosition(), length);
+ lastFragment.setSourceRange(lastFragment.getStartPosition(), length);
previousTag = (TagElement) lastFragment;
}
}
I would love to see these merged as part of this PR, whereas other findings can be conveniently handled separately, notably:
|
In the interest of getting ourselves unblocked on other issues, I have removed the ASTConverterMarkdownTest from the convert test suite and planning to release this today. The ASTConverterMarkdownTest takes after ASTConverterJavadocTest, which has another ad-hoc parser/scanner that parses the Javadoc and compares with the DOM output. I am yet to completely have that under control. Will take that up immediately after this. |
bc83ae1
to
4907326
Compare
What it does
Partially fixes #2738
How to test
Author checklist